feat(kotlin-sdk): bind every wallet's shielded sub-wallet for multi-wallet flows by QuantumExplorer · Pull Request #4046 · dashpay/platform

QuantumExplorer · 2026-07-08T20:49:36Z

Issue being fixed or feature implemented

Android port of #4038 (merged for SwiftExampleApp). The Kotlin app bound only the lexicographically-smallest wallet's shielded (Orchard) sub-wallet to the Rust sync coordinator (AppContainer.rebindWalletScopedServices → single shieldedService.bind). With multiple wallets loaded, every non-first wallet's shielded pool was invisible — no receive address, no note sync — blocking the multi-wallet shielded QA rows SH-14/15/16 and the substance of CORE-21.

Kotlin-only change: the Rust coordinator was already multi-wallet, and the Android per-wallet read surfaces (Receive sheet, wallet-detail balance, self-shield guard, identity-funding gate) already resolve per-wallet — they just returned nothing for wallets that were never registered.

What was done?

Same "single UI mirror + multi-engine-bind" design as iOS:

ShieldedService keeps mirroring ONE wallet (the first) for the global Sync tab via bind(...).
New ShieldedService.bindEngine(manager, walletId, dbPath, accounts) does engine-only registration (configure + bind, best-effort, no mirror repoint), returning success for fleet passes.
AppContainer.rebindWalletScopedServices() engine-binds every other loaded wallet through the pure, unit-tested engineBindOtherWallets seam — placed before the fallible sync-start calls so a start failure cannot leave non-mirror wallets unregistered (iOS review lesson).
No "already bound" fast path in bindEngine, on purpose: the only cheap probe (shieldedDefaultAddress) reflects the wallet-level sub-wallet binding, which survives clearShieldedStorage — skipping on it would leave post-Clear wallets permanently unregistered. This exact trap was hit and verified on-simulator during the iOS round.
Clear (SH-12) is now fleet-aware, fixing a latent bug the port would otherwise expose: clearShieldedStorage() empties the SHARED commitment tree, but the Room wipe was scoped to the bound wallet — a non-mirror wallet's surviving shielded_sync_states watermark would restore a position ahead of the emptied tree on re-bind and gate-skip every note (the known Room↔SQLite watermark-divergence freeze). Clear now wipes every wallet's shielded rows and re-binds the whole fleet (mirror via bind, others via bindEngine), regardless of the mirror bind's outcome.

Docs: TEST_PLAN.md rows SH-12/13/14/15/16 and CORE-21 updated (both wallets bound automatically; no wallet-swap needed).

How Has This Been Tested?

./gradlew :sdk:testDebugUnitTest :app:assembleDebug — BUILD SUCCESSFUL; all 4 new ShieldedEngineBindPlanTest cases pass (binds every non-mirror wallet, one failure doesn't stop the rest, single-wallet no-op, mirror skipped by value not position).
On-emulator two-wallet verification (distinct per-wallet tdash1… addresses, Shielded engine-bound logcat line for wallet B, one Room shielded_sync_states row per wallet after a pass) is in progress on a clean AVD; will report results in a comment. The identical design was fully verified on-simulator for iOS in feat(swift-example-app): bind every wallet's shielded sub-wallet for multi-wallet flows #4038.

Breaking Changes

None.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

🤖 Generated with Claude Code

…allet flows Android port of #4038 (SwiftExampleApp): engine-bind EVERY loaded wallet's shielded (Orchard) sub-wallet with the Rust sync coordinator, not just the lexicographically-smallest wallet. Unblocks SH-14/15/16 (cross-wallet shielded transfer / unshield / withdraw). Kotlin-only — the Rust coordinator was already multi-wallet, and the per-wallet READ surfaces (Receive sheet, balances, self-shield guard, identity funding) already read per-wallet. Same "single UI mirror + multi-engine-bind" design: ShieldedService keeps mirroring one wallet for the Sync tab via bind(); the new bindEngine() does engine-only registration, and AppContainer.rebindWalletScopedServices() runs the fleet pass through the pure engineBindOtherWallets seam, placed BEFORE the fallible sync-start calls. bindEngine has NO already-bound fast path on purpose: shieldedDefaultAddress reflects the wallet-level sub-wallet binding, which survives clearShieldedStorage, so skipping on it would leave post-Clear wallets permanently unregistered (iOS lesson, verified on-simulator there). Clear (SH-12) is fleet-aware too, fixing a latent multi-wallet bug the port would otherwise expose: clearShieldedStorage empties the SHARED commitment tree, but the Room wipe was scoped to the bound wallet — a non-mirror wallet's surviving watermark would restore a position ahead of the emptied tree on re-bind and gate-skip every note (the known Room↔SQLite watermark-divergence freeze). Clear now wipes every wallet's rows and re-binds the whole fleet: mirror via bind(), others via bindEngine(), regardless of the mirror bind's outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-08T20:50:44Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17eb32d1-a406-4309-ad79-0e594704243f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/android-multiwallet-shielded

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

thepastaclaw · 2026-07-08T20:51:06Z

✅ Review complete (commit 949ea30)

thepastaclaw

Code Review

Source: reviewers: opus/claude-general failed (extra usage quota; resets Jul 10, 8am America/Chicago), gpt-5.5/codex-general completed; verifier: gpt-5.5/codex; specialists: none.

Both verified findings are valid and in scope for PR #4046. The PR claims fleet-wide shielded binding and clear behavior, but the current implementation still has one over-broad Room wipe across networks and one ordering path where platform-address sync failure prevents the new engine-bind pass from running.

🔴 2 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/services/ShieldedService.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/services/ShieldedService.kt:463-468: Shielded clear wipes other networks' Room rows
  `clearShieldedStorage()` runs on the active `PlatformWalletManager`, whose shielded coordinator is network-scoped and opened from `shielded_tree_${manager.network.networkName}.sqlite`. These `deleteAll...` calls wipe every shielded row in the shared Room database, including wallets on other networks whose Rust shielded stores were not reset. Clearing shielded state on one network can therefore delete another network's notes, activity, and watermarks while leaving that network's Rust tree intact, causing data loss and Room/Rust divergence after switching networks.

In `packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/di/AppContainer.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/di/AppContainer.kt:211-214: Platform sync failure can skip shielded engine binds
  The PR description and inline comment require the multi-wallet engine-bind pass to run before fallible sync-start calls so a start failure cannot leave non-mirror wallets unregistered. This block checks and starts platform-address sync before `shieldedService.bind(...)` and `engineBindOtherWallets(...)`, inside the same `try`. If the platform-address running check or start call throws, control jumps to the catch and no shielded coordinator registration happens for the non-mirror wallets until a later rebind or relaunch. Move the platform-address start block after the shielded bind and engine-bind pass.

thepastaclaw · 2026-07-08T21:06:21Z

        db.withTransaction {
-            db.shieldedDao().deleteActivityByWallet(walletId)
-            db.shieldedDao().deleteOutgoingNotesByWallet(walletId)
-            db.shieldedDao().deleteNotesByWallet(walletId)
-            db.shieldedDao().deleteSyncStatesByWallet(walletId)
+            db.shieldedDao().deleteAllActivity()
+            db.shieldedDao().deleteAllOutgoingNotes()
+            db.shieldedDao().deleteAllNotes()
+            db.shieldedDao().deleteAllSyncStates()
        }


🔴 Blocking: Shielded clear wipes other networks' Room rows

clearShieldedStorage() runs on the active PlatformWalletManager, whose shielded coordinator is network-scoped and opened from shielded_tree_${manager.network.networkName}.sqlite. These deleteAll... calls wipe every shielded row in the shared Room database, including wallets on other networks whose Rust shielded stores were not reset. Clearing shielded state on one network can therefore delete another network's notes, activity, and watermarks while leaving that network's Rust tree intact, causing data loss and Room/Rust divergence after switching networks.

Suggested change

db.withTransaction {

db.shieldedDao().deleteActivityByWallet(walletId)

db.shieldedDao().deleteOutgoingNotesByWallet(walletId)

db.shieldedDao().deleteNotesByWallet(walletId)

db.shieldedDao().deleteSyncStatesByWallet(walletId)

db.shieldedDao().deleteAllActivity()

db.shieldedDao().deleteAllOutgoingNotes()

db.shieldedDao().deleteAllNotes()

db.shieldedDao().deleteAllSyncStates()

}

val activeWalletIds = db.walletDao()

.getByNetwork(mgr.network.ffiValue)

.map { it.walletId }

db.withTransaction {

for (id in activeWalletIds) {

db.shieldedDao().deleteActivityByWallet(id)

db.shieldedDao().deleteOutgoingNotesByWallet(id)

db.shieldedDao().deleteNotesByWallet(id)

db.shieldedDao().deleteSyncStatesByWallet(id)

}

}

_{source: ['codex']}

Resolved in e75ea53 — Shielded clear wipes other networks' Room rows no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

…e fallible sync starts Addresses the two blocking review findings on #4046. - ShieldedService.clearLocalState: the Rust reset is network-scoped (`shielded_tree_<network>.sqlite`), so a global `deleteAll*` Room wipe dropped OTHER networks' notes/activity/watermarks while their Rust trees stayed populated → data loss + Room/Rust divergence after a network switch. Wipe only the active network's fleet (`mgr.wallets`, which is network-scoped) via the per-wallet `delete*ByWallet` DAO methods. - AppContainer.rebindWalletScopedServices: moved the fallible platform-address sync start to AFTER the shielded bind + engine-bind pass. Previously a throw from the platform-address running-check/start jumped to the catch and left the non-mirror wallets' shielded coordinators unregistered until a later rebind/relaunch — matching what the existing comment already promised. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

QuantumExplorer · 2026-07-09T17:32:50Z

Both blocking findings addressed in e75ea53:

Shielded clear wiping other networks — clearLocalState now scopes the Room wipe to the active network's fleet (mgr.wallets, which is network-scoped) via the per-wallet delete*ByWallet DAO methods instead of the global deleteAll*. The Rust reset only empties this network's shielded_tree_<network>.sqlite, so other networks' notes/activity/watermarks are left intact — no more Room/Rust divergence on a network switch.
Platform sync failure skipping engine binds — moved the fallible startPlatformAddressSync() to run after the bind + engineBindOtherWallets pass, so a throw from the platform-address check/start can no longer jump to the catch and leave the non-mirror wallets' shielded coordinators unregistered (the code now matches what the comment already promised).

:sdk + :app compileDebugKotlin and :sdk testDebugUnitTest are green. Also validated end-to-end on-device (via a cherry-pick of this branch's commit) before the fixes: the multi-wallet bind enables SH-14 (shielded transfer docdri→sh03test, recipient received 0.002 DASH) and SH-17 (both wallets bound concurrently with distinct balances).

thepastaclaw

Code Review

Source: reviewers=general:claude/opus(failed:extra-usage), general:codex/gpt-5.5(failed:auth), ffi-engineer:claude/opus(failed:extra-usage), ffi-engineer:codex/gpt-5.5(failed:auth), fallback openclaw general+ffi (ok); verifier=openclaw/manual-code-check(ok).
Reviewed commit: e75ea53e. Prior reviewed commit: 5baf9ac1.

Carried-forward prior findings: none. Both prior blockers are FIXED at current head.
New findings in latest delta 5baf9ac1..e75ea53e: one residual suggestion (Clear only iterates currently loaded same-network wallets) and one KDoc nit.

The latest push scopes the Room wipe to mgr.wallets with per-wallet deletes and moves platform-address sync start after mirror bind + engine-bind, matching the review fixes claimed in the commit message.

🟡 1 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/services/ShieldedService.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/services/ShieldedService.kt:469-476: Clear still misses unloaded same-network shielded rows
  The cross-network wipe is fixed: `clearLocalState` now deletes only wallets from the network-scoped `mgr.wallets` map via per-wallet DAO methods. However, `loadPersistedWallets()` only publishes wallets that successfully `getWallet`, so a same-network wallet that still has Room shielded rows (failed restore / not currently loaded) is skipped by this loop. After `clearShieldedStorage()` empties the shared per-network tree, leftover watermarks/notes for those unloaded wallets can later re-enter through `restore_for_wallet` and recreate the Room↔tree divergence freeze Clear is meant to prevent. Prefer wiping every Room wallet on `mgr.network` (for example `walletDao().getByNetwork(mgr.network.ffiValue)`), not only the currently loaded fleet.

thepastaclaw · 2026-07-09T17:48:16Z

        db.withTransaction {
-            db.shieldedDao().deleteActivityByWallet(walletId)
-            db.shieldedDao().deleteOutgoingNotesByWallet(walletId)
-            db.shieldedDao().deleteNotesByWallet(walletId)
-            db.shieldedDao().deleteSyncStatesByWallet(walletId)
+            for (other in mgr.wallets.value.values) {
+                val wid = other.walletId
+                db.shieldedDao().deleteActivityByWallet(wid)
+                db.shieldedDao().deleteOutgoingNotesByWallet(wid)
+                db.shieldedDao().deleteNotesByWallet(wid)
+                db.shieldedDao().deleteSyncStatesByWallet(wid)
+            }


🟡 Suggestion: Clear still misses unloaded same-network shielded rows

The cross-network wipe is fixed: clearLocalState now deletes only wallets from the network-scoped mgr.wallets map via per-wallet DAO methods. However, loadPersistedWallets() only publishes wallets that successfully getWallet, so a same-network wallet that still has Room shielded rows (failed restore / not currently loaded) is skipped by this loop. After clearShieldedStorage() empties the shared per-network tree, leftover watermarks/notes for those unloaded wallets can later re-enter through restore_for_wallet and recreate the Room↔tree divergence freeze Clear is meant to prevent. Prefer wiping every Room wallet on mgr.network (for example walletDao().getByNetwork(mgr.network.ffiValue)), not only the currently loaded fleet.

Suggested change

db.withTransaction {

db.shieldedDao().deleteActivityByWallet(walletId)

db.shieldedDao().deleteOutgoingNotesByWallet(walletId)

db.shieldedDao().deleteNotesByWallet(walletId)

db.shieldedDao().deleteSyncStatesByWallet(walletId)

for (other in mgr.wallets.value.values) {

val wid = other.walletId

db.shieldedDao().deleteActivityByWallet(wid)

db.shieldedDao().deleteOutgoingNotesByWallet(wid)

db.shieldedDao().deleteNotesByWallet(wid)

db.shieldedDao().deleteSyncStatesByWallet(wid)

}

val activeWalletIds = db.walletDao()

.getByNetwork(mgr.network.ffiValue)

.map { it.walletId }

db.withTransaction {

for (wid in activeWalletIds) {

db.shieldedDao().deleteActivityByWallet(wid)

db.shieldedDao().deleteOutgoingNotesByWallet(wid)

db.shieldedDao().deleteNotesByWallet(wid)

db.shieldedDao().deleteSyncStatesByWallet(wid)

}

}

_{source: ['openclaw-ffi-engineer']}

Resolved in 949ea30 — Clear still misses unloaded same-network shielded rows no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

thepastaclaw · 2026-07-09T17:48:16Z

+     *    the bound wallet: with multi-engine-bind, every loaded wallet has
+     *    rows, and [clearShieldedStorage] empties the SHARED tree — a
+     *    non-mirror wallet whose Room watermark survived would restore a
+     *    position ahead of the now-empty tree on re-bind and gate-skip
+     *    every note (the exact Room↔SQLite watermark-divergence freeze
+     *    this Clear exists to fix).
     * 3. **Re-bind + restart sync** — [clearShieldedStorage] dropped every


💬 Nitpick: clearLocalState KDoc still says wipe is GLOBAL

Step 2 of the clearLocalState KDoc still describes the Room wipe as deliberately GLOBAL / every wallet's rows, but the implementation is now network-fleet scoped and explicitly avoids other networks. Update the doc to match the per-network contract so future edits don't reintroduce deleteAll*.

_{source: ['openclaw-general', 'openclaw-ffi-engineer']}

Resolved in 949ea30 — clearLocalState KDoc still says wipe is GLOBAL no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

…lets Follow-up to the review on the per-network Clear fix: - Wipe by `walletDao().getByNetwork(mgr.network.ffiValue)` instead of `mgr.wallets`, which only holds the successfully-loaded fleet. A same-network wallet that failed restore still has Room shielded rows; the loaded-only loop skipped it, so after clearShieldedStorage empties the shared per-network tree its leftover watermark could re-enter via restore_for_wallet and recreate the Room/tree freeze Clear is meant to prevent. - Update the clearLocalState KDoc (step 2) to describe the per-network contract instead of the old "deliberately GLOBAL" wording, so a future edit doesn't reintroduce deleteAll*. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

QuantumExplorer · 2026-07-09T17:56:55Z

Follow-up review addressed in 949ea30:

🟡 Clear still missed unloaded same-network rows — the wipe now iterates walletDao().getByNetwork(mgr.network.ffiValue) rather than the loaded mgr.wallets fleet, so a same-network wallet that failed restore (still holding Room shielded rows) is also wiped. That closes the re-freeze path where its leftover watermark could re-enter via restore_for_wallet after the shared per-network tree is emptied.
💬 KDoc still said GLOBAL — clearLocalState step 2 now documents the per-network contract (scoped via getByNetwork, never deleteAll*).

:sdk + :app compileDebugKotlin and :sdk testDebugUnitTest green.

thepastaclaw

Code Review

Incremental review of 949ea30 vs prior head e75ea53. Both prior findings (Room wipe missing unloaded same-network wallets; KDoc mislabeling the wipe as GLOBAL) are FIXED and moved to resolved_findings. No new findings in the latest delta. Codex agents failed to run due to a refresh-token auth error; reconciliation is based on the two Claude agents plus direct verification of ShieldedService.kt and WalletDao.kt at head. No carried-forward findings.

thepastaclaw suggested changes Jul 8, 2026

View reviewed changes

thepastaclaw reviewed Jul 9, 2026

View reviewed changes

thepastaclaw approved these changes Jul 9, 2026

View reviewed changes

QuantumExplorer merged commit 8adf257 into feat/kotlin-sdk-and-example-app Jul 9, 2026
4 checks passed

QuantumExplorer deleted the claude/android-multiwallet-shielded branch July 9, 2026 18:26

Uh oh!

Conversation

QuantumExplorer commented Jul 8, 2026

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

thepastaclaw commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Code Review

Uh oh!

thepastaclaw Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

QuantumExplorer commented Jul 9, 2026

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

thepastaclaw Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

QuantumExplorer commented Jul 9, 2026

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

thepastaclaw commented Jul 8, 2026 •

edited

Loading

thepastaclaw left a comment •

edited

Loading